Migrate DurableTask.ServiceBus Table and Blob store SDK from WindowsAzure.Storage to Azure.Data.Tables and Azure.Storage.Blobs #1112
Conversation
There was a problem hiding this comment.
A few preliminary questions. The changes make sense at first glance, but I see many instances where properties are marked with "IgnoreDataMember" only to be replaced by slightly differently-named properties below; I'm unsure why we cannot just change the implementation of the pre-existing properties.
Thank you for this PR!
| /// Creates a new AzureStorageBlobStore using the supplied hub name and cloud storage account | ||
| /// Creates a new AzureStorageBlobStore using the supplied hub name, endpoint and token credential |
There was a problem hiding this comment.
Just confirming - does this mean that only managed identity (i.e token credentials, right?) would be supported now?
There was a problem hiding this comment.
To preserve previous functionality, we are maintaining support for connection strings. The breaking change is to introduce support for token credentials and removing support for the "cloud storage account" as that class no longer exists in the new Azure SDK.
|
|
||
| /// <summary> | ||
| /// Gets or sets the entity etag | ||
| /// </summary> | ||
| public string ETag { get; set; } |
There was a problem hiding this comment.
dropping these (and other public fields) is technically a breaking change. Just jotting this down for down, still unsure what replaces this (or if functionality was dropped)
There was a problem hiding this comment.
This is being replaced with an ETag class provided by Azure Core, and a required part of the new Azure SDK. While unfortunate this is a breaking change, this should not change functionality in a substantial manner.
src/DurableTask.ServiceBus/Tracking/AzureTableOrchestrationHistoryEventEntity.cs
Outdated
Show resolved
Hide resolved
src/DurableTask.ServiceBus/Tracking/AzureTableOrchestrationHistoryEventEntity.cs
Outdated
Show resolved
Hide resolved
| // In order to maintain table schema with the new Azure SDK, we need the following accessor. | ||
| // As a result, this HistoryEventJson does not need to be documented. | ||
| [DataMember(Name = "HistoryEvent")] | ||
| public string HistoryEventJson |
There was a problem hiding this comment.
I'm a bit confused - why can't these getter and setter be part of the HistoryEvent declared above?
There was a problem hiding this comment.
To preserve functionality, I thought it best to allow the HistoryEvent object be accessed via default getters and setters. This HistoryEventJson property only exists to allow the entity to cooperate with the new Azure SDK, as table read and writes for entities now behave differently.
src/DurableTask.ServiceBus/Tracking/AzureTableOrchestrationJumpStartEntity.cs
Outdated
Show resolved
Hide resolved
…toryEventEntity.cs Co-authored-by: David Justo <[email protected]>
src/DurableTask.ServiceBus/Tracking/AzureTableOrchestrationStateEntity.cs
Outdated
Show resolved
Hide resolved
src/DurableTask.ServiceBus/Tracking/AzureTableOrchestrationHistoryEventEntity.cs
Outdated
Show resolved
Hide resolved
src/DurableTask.ServiceBus/Tracking/AzureTableOrchestrationJumpStartEntity.cs
Outdated
Show resolved
Hide resolved
src/DurableTask.ServiceBus/Tracking/AzureTableOrchestrationStateEntity.cs
Outdated
Show resolved
Hide resolved
src/DurableTask.ServiceBus/Tracking/AzureTableOrchestrationStateEntity.cs
Outdated
Show resolved
Hide resolved
Notable Changes
Validation